Skip to content

nvx-rs: add the VMM supervisor with a spawn allowlist, stop, and reattach - #656

Merged
Enrique Saurez (esaurez) merged 10 commits into
devfrom
esaurez/a1-host-h3-supervisor-20261020
Oct 21, 2026
Merged

Enrique Saurez (esaurez) merged 10 commits into
devfrom
esaurez/a1-host-h3-supervisor-20261020

Conversation

@esaurez

@esaurez Enrique Saurez (esaurez) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Step H3 of #620: one supervisor of a sandbox's VMM, nvx::host::supervisor, beneath the CLI, host::instance, and external orchestrators. It launches OpenVMM through the launch journal of the sandbox store from #628, passes it only the descriptors or handles that the caller allows, waits for its exit by event, stops it through the caller's graceful step with a bounded forced step on request, and reattaches to it after the orchestrator restarts within the same host boot. Callers keep the configuration, the launch planning, the authorization, and the application protocol: the supervisor owns no control session.

What changed

  • Spawn allowlist (supervisor::spawn, VmmCommand, Inherited): the caller names the descriptors (Linux) or handles (Windows) that OpenVMM inherits besides its standard streams, which are the capability pipe on standard input and the claimed log on standard output and error. The VMM inherits each one under the number that it has in the caller, so the caller can name it in OpenVMM's arguments, for example as /proc/self/fd/N (Inherited::proc_path), and inherits nothing else of the caller's.
    • On Linux, the descriptors stay close-on-exec in the caller. Between its fork and its exec, the child marks every descriptor close-on-exec, as before, and then clears the flag of the allowed ones only, so a process that another thread spawns meanwhile inherits none of them. A descriptor that is not close-on-exec in the caller, which any process that the caller spawns would inherit, is refused, as are standard streams.
    • On Windows, the allowed handles join the explicit PROC_THREAD_ATTRIBUTE_HANDLE_LIST of platform::spawn_detached, once each, and are inheritable only while the VMM is created: a guard makes each one inheritable and restores its earlier flag right after CreateProcessW, as it does for the standard handles. Spawns that allow handles take that window one at a time, so several may allow the same handle. The job breakaway is unchanged. A process that another thread creates in that window without a handle list, as std::process::Command does, can inherit them too; Inherited documents it.
    • Other hosts refuse any allowed descriptor.
  • Launch (supervisor::launch, Starting): runs the store's launch steps under the sandbox's lock: LockedStore::begin_launch, the caller's command for the endpoint that the launch chose, Launch::take_log, the spawn, and Launch::record_process right after it. The caller checks through its own transport that the VMM serves its endpoint; then Starting::commit writes the runtime record and returns a Vmm, or Starting::abandon ends the launch as Launch::abandon does. A step that fails ends the launch, after it ends a VMM whose identity it could not record through the child's handle, and says so when the launch remains for recovery. Dropping a Starting abandons it, and a crash leaves the journal for LockedStore::recover.
  • Exit waits (Vmm::wait, Vmm::has_exited): the Process of nvx-rs: add verified process references with event-driven exit waits #625, a pidfd on Linux and the process handle on Windows, with polling only on Linux hosts without pidfds.
  • Stop (Vmm::stop): one graceful step, a callback of the caller's that receives the VM and the deadline, then a wait for the VMM's exit event until the deadline. A forced step, only when the caller requests it with a bound, terminates the VMM through its verified handle and waits up to that bound; without it, the stop never signals the VMM. Stopped reports Graceful, Forced, or TimedOut, and the error of a graceful step that failed, which the stop does not wait out. A VMM that exited before the stop skips the graceful step. Once the VMM has exited, the stop clears its runtime record, unless the record names another launch by then. The forced bound is checked like the crate's other timeouts, before anything runs.
  • Reattach (supervisor::reattach): checks the caller's runtime identity, ends an interrupted launch with LockedStore::recover, reads the runtime record, holds the VMM with Process::open, and compares identities with RuntimeRecord::check_identity. It reports Live; Mismatched, with the IdentityParts that differ; Exited, with the record that it cleared and the recovery; or Indeterminate, with the reason, keeping the runtime record and the journal of an interrupted launch for a later reattach. Reattach signals no process that it has not verified, though recovery may have terminated a verified VMM of an interrupted launch that has not exited yet. A record of an earlier boot, or one whose process ID now names another process, names a VMM that exited. Reattach doesn't replay Start or MAPS, restore lost streams, or resolve images.
  • Recovery and concurrent spawns (store): on Linux, a process holds a copy of each descriptor of its parent from its fork until it executes its program. So a spawn by another thread, such as the launch of another sandbox's VMM, held a launch's claim on its log for that moment, and recovery, or the abandon of a failed launch, reported the launch indeterminate and kept its journal. Recovery now checks a held claim again for up to a second before it concludes that a process of the launch may still run.
  • The older helpers keep working until H4 moves the CLI and host::instance, which are unchanged: process::spawn, detach_child, and kill_child go through the supervisor with an empty allowlist, and Launched moved to the supervisor, which process re-exports. The store's documentation now points to the supervisor.

Tests

  • nvx-rs/tests/supervisor.rs, which nvx-rs: add verified process references with event-driven exit waits #625 started, also runs the test executable as an orchestrator that launches through the supervisor and crashes at each launch step:
    • before the spawn: reattach reports the launch exited;
    • between the spawn and the identity: a VMM stand-in that serves no endpoint is never signaled, and reattach stays indeterminate until it exits; with the testing feature, the fake OpenVMM, which serves its endpoint, is verified and ended;
    • before the endpoint: the recorded VMM, which isn't the caller's child, is ended through its verified identity.
  • A committed VMM outlives its orchestrator; the restarted orchestrator reattaches to it and stops it gracefully, which clears its record. A VMM of another runtime is reported mismatched: a stop without the forced step times out and leaves it running with its record, and a requested forced stop reports forced. Runtime records with a reused process ID or an earlier boot are reported exited, and the process that has the ID isn't signaled.
  • The allowlist test launches VMMs that inherit one pipe each, while another thread spawns through the supervisor and through std::process with a pipe that it makes inheritable. On Linux, each VMM holds exactly descriptors 0, 1, and 2, with the log on 1 and 2, and its pipe under the caller's number, and no process of the other thread holds a VMM's pipe. On Windows, each VMM holds its pipe under the caller's value, and none of the handles of the other thread's spawns.
  • On Windows: a VMM that breaks away survives its orchestrator's job, one that doesn't ends with it, and a VMM whose identity this user may not query is reported indeterminate and left running.
  • Three tests that checked a released lock or claim with a single try, two of the store's and the instance lock test, retry for a bounded time, since a process that a concurrent test spawns holds the lock or claim until it executes its program.
  • Unit tests cover the stop decisions against a scripted VMM; the reattach results, with this test process as a held VMM, including an identity that cannot be examined, which the Linux integration tests cannot produce without privileges; the early failures of a launch; the descriptor numbers on Linux; and the inheritance of Windows handles while they are inheritable.

Not in this PR: the delegation of the CLI and host::instance (H4), and the test adapter's host half and the restart acceptance (H5).

Part of #620. Refs #535.

Validation

  • Windows 11, x64: python scripts\nvx.py verify and python scripts\check_markdown_links.py. In nvx-rs, with NVX_BUNDLE=skip: cargo fmt --all --check, cargo clippy --locked --all-targets --all-features -- -D warnings, cargo clippy --locked --lib --no-default-features -- -D warnings, cargo test --locked --all-features, cargo test --locked, cargo doc --locked --no-deps --all-features with RUSTDOCFLAGS=-D warnings, and cargo +1.89 check --locked --all-targets --all-features. The same steps for the aci_edge_sandboxes workspace, and python scripts\nvx.py test-agent-bundling --toolchain 1.95.0, as check-aci-edge-sandboxes runs them.
  • Linux (Ubuntu on WSL 2): the same steps for both crates, plus cargo check --locked --all-targets --all-features --target aarch64-apple-darwin. The tests ran as an unprivileged user, since the recorded --mount-owner caller scenarios of tests/cli.rs refuse a root-owned share.
  • The runtime adapter (tests/runtime-adapter), on both: the steps of check-nvx-rs for its protocol-only and host-only configurations, and, on Linux, for its x86_64-unknown-linux-musl guest-only configuration.
  • tests/supervisor.rs passed 10 runs in a row on each OS, on 853ddce. Without the lock of the Windows spawns that allow handles, its new unit test failed 4 runs out of 4.
  • Real VMs, WHP (Windows 11), on 8bf8b9a, with NVX_RS naming a release build of it and the newest release that python scripts\nvx.py download --hypervisor whp installs, v0.1.0-dev.dad3e41d7f05: python scripts\nvx.py test-microvm --backend whp, its default scenarios; the same with --scenario sandbox-lifecycle --scenario managed-exec-config, which launch through nvx.py sandbox, and so through the supervisor's spawn; and python scripts\nvx.py test-aci-edge-sandboxes --backend whp, which launches through host::instance. All passed.
  • KVM (WSL 2): not run. OpenVMM's --cpu-profile auto selects the built-in profile of this host's CPU, which nested KVM refuses (MSR 0x10a bits 8, 17 are not supported) before any VM boots, and test-microvm takes no host profile.
  • The race of the store's tests: on a 24-thread Linux host, the library tests of 853ddce, with --all-features, failed in 20 runs out of 20, against none of 20 at its base, b47a9f8, and none of 8 when they skipped the new host::supervisor tests. A child that a host::supervisor test forked held other tests' locks and logs for about 5 ms before its exec. With the fix, 8bf8b9a passed 20 runs at the default thread count and 20 with 48 test threads, and so did c7b7c94.
  • All of the above ran on 8bf8b9a, after the rebase onto dev at 916a61f, past tests: serve the control session in the runtime adapter's guest agent #640, nvx-rs: run the control session on the shared NVXS session's host role #641, and doc: map the OpenVMM upstream roadmap onto the microvm/v2.2 chain #667. The rebase resolved two documentation conflicts with nvx-rs: run the control session on the shared NVXS session's host role #641, in nvx-rs/README.md and nvx-rs/src/host/mod.rs, keeping both sides; the code of the earlier commits is unchanged.
  • After the final review, c7b7c94 changes only the instance lock test: on it, the nvx-rs steps above ran again on both OSes, and CI passed on it, in run 39542215794.
  • After human review, 3c9deee to 8b0bf90 name the last standard stream descriptor, recheck a held log claim at the store's 25 ms lock retry interval instead of 10 ms, share the unit tests' wait for a released lock, and document process::POLL_INTERVAL. On them, all of the Windows and Linux steps above ran again, and the Linux library tests passed 20 runs at the default thread count and 20 with 48 test threads.
  • Left to CI: KVM and MSHV, the WHP matrix on the CI runners, the guest-binary checks, and the bundling check of a synthetic release.

Copilot AI balanced review requested due to automatic review settings October 20, 2026 20:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Concurrent Windows spawns sharing a handle can disrupt inheritance and leave that handle inheritable afterward.

2 open issues
What changed in this PR

Adds the shared VMM supervisor for #620’s H3, building on verified process references and the sandbox store while preserving existing callers.

Changes:

  • Adds explicit descriptor and handle inheritance allowlists.
  • Implements journaled launch, bounded stop, and restart reattachment.
  • Expands lifecycle, crash-recovery, and inheritance tests.
File Description
nvx-rs/​tests/​supervisor.rs Tests recovery, reattachment, stops, and inheritance.
nvx-rs/​tests/​managed_lifecycle.rs Updates a comment’s supervisor reference.
nvx-rs/​src/​host/​sys/​process.rs Enables allowed descriptors before exec.
nvx-rs/​src/​host/​supervisor/​tests.rs Tests supervisor decisions and launch failures.
nvx-rs/​src/​host/​supervisor/​mod.rs Adds launch, stop, and reattachment APIs.
nvx-rs/​src/​host/​supervisor/​child.rs Implements allowlisted spawning and child management.
nvx-rs/​src/​host/​store/​mod.rs Documents supervisor integration.
nvx-rs/​src/​host/​process.rs Delegates legacy launch helpers.
nvx-rs/​src/​host/​platform/​windows.rs Adds handle allowlisting and flag restoration.
nvx-rs/​src/​host/​platform/​other.rs Rejects unsupported inheritance requests.
nvx-rs/​src/​host/​platform/​linux.rs Validates and forwards descriptor allowlists.
nvx-rs/​src/​host/​mod.rs Exposes and documents the supervisor.
nvx-rs/​README.md Documents supervisor APIs and tests.
nvx-rs/​Cargo.toml Updates the Windows test-dependency comment.

Review effort: Balanced

Comment thread nvx-rs/src/host/platform/windows.rs
Comment thread nvx-rs/src/host/supervisor/mod.rs Outdated
Copilot AI balanced review requested due to automatic review settings October 20, 2026 20:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-platform inheritance and crash recovery need final human review, with an unresolved containment concern and backend validation pending.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Reject already-inheritable descriptors before spawning

nvx-rs/​src/​host/​platform/​linux.rs:267

An allowed descriptor may already be inheritable, for example if native code opened it without O_CLOEXEC. The child-only flag changes leave it inheritable in the caller, so an unrelated concurrent std::process::Command can receive it despite the documented containment guarantee. Reject descriptors lacking FD_CLOEXEC before spawning, rather than temporarily changing the caller's flag, and add a test using an already-inheritable descriptor.

2 resolved since last review

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 20, 2026 20:29
@esaurez

Copy link
Copy Markdown
Contributor Author

The review of 8ff6af6 also listed, in its body only, a previously missed issue: on Linux, an allowed descriptor that is not close-on-exec in the caller reaches every process that the caller spawns, not only the VMM. Fixed in 4df5103: platform::detach now refuses such a descriptor with InvalidInput before anything runs, and leaves its flag as it was, and the docs of Inherited, spawn, and the README say so. The new unit test descriptors_that_are_not_close_on_exec_are_refused covers it.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-platform inheritance, termination, and crash recovery require final human review, with real-VM validation still pending.

No open issues 🎉

Review effort: Balanced

@esaurez
Enrique Saurez (esaurez) marked this pull request as ready for review October 20, 2026 20:38
@esaurez
Enrique Saurez (esaurez) force-pushed the esaurez/a1-host-h3-supervisor-20261020 branch from 4df5103 to 853ddce Compare October 21, 2026 00:51
Copilot AI balanced review requested due to automatic review settings October 21, 2026 00:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-platform inheritance, crash recovery, and verified termination require final human review and platform CI validation.

No open issues 🎉

Review effort: Balanced

Enrique Saurez and others added 5 commits October 20, 2026 19:47
…tach

Add `nvx::host::supervisor`, beneath the CLI, `host::instance`, and
external orchestrators. It launches OpenVMM through the launch journal
of the sandbox store, passes it only the descriptors or handles that the
caller allows, waits for its exit by event, stops it through the
caller's graceful step with a bounded forced step on request, and
reattaches to it after the orchestrator restarts within the same host
boot.

On Linux, only the VMM's own process clears the close-on-exec flag of
the allowed descriptors, so a spawn on another thread inherits none of
them. On Windows, the allowed handles join the spawn's explicit handle
list, and are inheritable only while the VMM is created.

The spawn helpers of `host::process` now go through the supervisor with
an empty allowlist, until the CLI and `host::instance` move to it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03
Two spawns that allowed the same handle could each read the other's
temporary inheritability as the handle's own: one could then find the
handle no longer inheritable when it created its process, and leave it
inheritable for good. Spawns that allow handles now make them
inheritable, create their process, and restore the handles one at a
time.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03
Reattach signals no process that it has not verified, but the recovery
that it runs may have terminated a verified VMM that has not exited
yet, and removes a journal that a committed runtime record supersedes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03
A descriptor that is not close-on-exec in the caller reaches every
process that the caller spawns, not only the VMM, so a spawn that
allows one could not keep the allowlist's promise that no other spawn
inherits it. The spawn now refuses such a descriptor, before anything
runs, and leaves its flag as it was.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03
On Linux, a process holds a copy of each descriptor of its parent from its
fork until it executes its program. A spawn by another thread of a process
that holds a launch's log, such as the launch of another sandbox's VMM,
therefore holds that launch's claim on its log, and any sandbox lock that the
process holds, for that moment. Recovery, and the abandon of a failed launch,
then reported a launch that nothing of it held anymore as indeterminate, and
kept its journal.

Recovery now checks a held claim again for up to a second before it
concludes that a process of the launch may still run. The store's tests that
take a released lock with a deadline that has passed, or check a released
claim directly, retry for a bounded time too.

The supervisor's spawning tests run next to the store's tests, which made
this race fail the library tests on CI, and in every one of 20 runs on a
24-thread host. With this change, 50 runs pass, 30 of them with 48 test
threads.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03
@esaurez
Enrique Saurez (esaurez) force-pushed the esaurez/a1-host-h3-supervisor-20261020 branch from 853ddce to 8bf8b9a Compare October 21, 2026 03:14
Copilot AI balanced review requested due to automatic review settings October 21, 2026 03:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-platform inheritance, process termination, and crash recovery warrant final human review despite no confirmed new defects.

No open issues 🎉

Review effort: Balanced

The instance lock test took its lock again with a single try right after
the holder released it. On Linux, a process that a concurrent test forks
while the holder has the lock holds a copy of it until the process
executes its program, so that try could still find the lock held. The
supervisor's spawning tests run in the same test binary.

The test now retries a lock that would block for up to ten seconds, as the
store's tests do, and still fails at once on any other error.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03
Copilot AI balanced review requested due to automatic review settings October 21, 2026 04:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Security-sensitive inheritance and crash-recovery termination across Linux and Windows warrant final human review and cross-platform CI validation.

No open issues 🎉

Review effort: Balanced

Comment thread nvx-rs/src/host/platform/linux.rs Outdated
Comment thread nvx-rs/src/host/store/mod.rs Outdated
Comment thread nvx-rs/src/host/store/mod.rs
Comment thread nvx-rs/src/host/store/tests.rs Outdated
Comment thread nvx-rs/src/host/process.rs
Enrique Saurez and others added 4 commits October 21, 2026 09:39
The VMM allowlist refused descriptors 0 to 2, and the hook of a detached
spawn marked the descriptors from 3 up close-on-exec, each through a bare
number. A documented constant now names that boundary and says why it is
there: a spawn gives the new process its own standard streams under those
numbers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03
Recovery checked a held claim on a launch's log again every 10 ms, an
interval of its own. It now waits at the 25 ms interval at which the store
already retries its lock, the interval of the crate's process and lock
polls, and no longer sleeps past its deadline. The documentation of the
one-second wait now says why it lasts that long, and which operations can
wait.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03
The store's tests and the instance lock test each waited for a lock or
claim that they had released with a loop of their own and literal
durations. They now share one helper, whose wait and retry interval are
named and documented.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03
The interval's documentation said only that process shutdown and identity
are polled at it. It now names the waits that poll, because no exit event
serves them, and what the interval costs them.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03
Copilot AI balanced review requested due to automatic review settings October 21, 2026 16:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-platform inheritance and crash-recovery guarantees warrant final human review despite no confirmed blocking findings.

No open issues 🎉

Review effort: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants